Skip to content

Improvements to align CTS and Spec for Virtual Memory #2272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 12, 2024
Merged

Improvements to align CTS and Spec for Virtual Memory #2272

merged 1 commit into from
Dec 12, 2024

Conversation

@martygrant
Copy link
Contributor

@martygrant martygrant commented Oct 31, 2024 •
edited
Loading

intel/llvm#15965

  • UR_RESULT_ERROR_INVALID_ENUMERATION test for urVirtualMemSetAccess
  • Add test for urVirtualMemMap with different access flags
  • Add test for urVirtualMemSetAccess with different access flags
  • InvalidEnumeration test for urPhysicalMemCreate and changed some tests to only run once - different size values were not needed for these
  • Add test for urPhysicalMemCreate with different flags (only one placeholder is available, this is just a fixture for future additions)
  • Add new urPhysicalMemGetInfo entry point along with a reference count enum. Added adapter implementations for this
  • Add a test for urPhysicalMemGetInfo and modified urPhysicalMemRelease/Retain to use GetInfo to verify reference count is updated accordingly

@github-actions github-actions bot added loader Loader related feature/bug conformance Conformance test suite issues. specification Changes or additions to the specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues labels Oct 31, 2024
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch 2 times, most recently from 2dde2cb to 25b5bfe Compare November 1, 2024 17:21
@martygrant martygrant mentioned this pull request Nov 1, 2024
Merged
@martygrant martygrant marked this pull request as ready for review November 4, 2024 09:46
@martygrant martygrant requested review from a team as code owners November 4, 2024 09:46
@martygrant martygrant requested a review from GeorgeWeb November 4, 2024 09:46
GeorgeWeb
GeorgeWeb approved these changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@GeorgeWeb GeorgeWeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CUDA and HIP adapter changes look good.

@kbenzie kbenzie requested a review from steffenlarsen November 6, 2024 12:01
@kbenzie
Copy link
Contributor

kbenzie commented Nov 6, 2024

@steffenlarsen could you have a look at the spec updates please?

steffenlarsen
steffenlarsen approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable addition to me. 👍

kbenzie
kbenzie reviewed Nov 6, 2024
View reviewed changes
RossBrunton
RossBrunton reviewed Nov 6, 2024
View reviewed changes
RossBrunton
RossBrunton reviewed Nov 6, 2024
View reviewed changes
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 25b5bfe to e825f58 Compare November 6, 2024 17:43
@martygrant martygrant mentioned this pull request Nov 6, 2024
Closed
14 tasks
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from ade8572 to a64c93f Compare November 7, 2024 12:07
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from a64c93f to 5ba5c2f Compare November 14, 2024 15:02
@martygrant martygrant mentioned this pull request Nov 14, 2024
Merged
nrspruit
nrspruit approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for L0

@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch 5 times, most recently from db6e1a6 to 3706cbc Compare November 21, 2024 08:56
@martygrant
Copy link
Contributor Author

martygrant commented Nov 21, 2024

@oneapi-src/unified-runtime-native-cpu-write could someone take a look at this PR please?
@oneapi-src/unified-runtime-cuda-write would someone mind taking an additional look at the CUDA adapter changes please? I made a small change after @GeorgeWeb approved the changes before

frasercrmck
frasercrmck reviewed Nov 21, 2024
View reviewed changes
aarongreig
aarongreig reviewed Nov 21, 2024
View reviewed changes
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 3706cbc to 6779ec3 Compare November 22, 2024 09:30
@github-actions github-actions bot added the opencl OpenCL adapter specific issues label Nov 22, 2024
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 6779ec3 to dcb99fd Compare November 22, 2024 09:51
aarongreig
aarongreig approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@aarongreig aarongreig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT save for a couple of points

@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from dcb99fd to bc72267 Compare November 22, 2024 11:19
PietroGhg
PietroGhg approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Native CPU lgtm, thank you

@martygrant martygrant added the ready to merge Added to PR's which are ready to merge label Nov 25, 2024
@martygrant
Improvements to align CTS and Spec for Virtual Memory:
800b452 
- UR_RESULT_ERROR_INVALID_ENUMERATION test for urVirtualMemSetAccess
- Add test for urVirtualMemMap with different access flags
- Add test for urVirtualMemSetAccess with different access flags
- InvalidEnumeration test for urPhysicalMemCreate and changed some tests to only run once - different size values were not needed for these
- Add test for urPhysicalMemCreate with different flags (only one placeholder is available, this is just a fixture for future additions)
- Add new urPhysicalMemGetInfo entry point along with a reference count enum. Added adapter implementations for this
- Add a test for urPhysicalMemGetInfo and modified urPhysicalMemRelease/Retain to use GetInfo to verify reference count is updated accordingly
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from bc72267 to 800b452 Compare December 12, 2024 09:38
@martygrant martygrant merged commit 6d4eec8 into oneapi-src:main Dec 12, 2024
73 checks passed
martygrant added a commit to martygrant/llvm that referenced this pull request Dec 12, 2024
@martygrant
[UR] Update tag to 6d4eec8 for:
7d878cc 
- oneapi-src/unified-runtime#2272
- oneapi-src/unified-runtime#2336
martygrant added a commit to intel/llvm that referenced this pull request Dec 13, 2024
@martygrant
[UR] Update tag to 6d4eec8 for UR#2272 and UR#2336 (#15965)
a37bba8 
oneapi-src/unified-runtime#2272
oneapi-src/unified-runtime#2336
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nrspruit nrspruit nrspruit approved these changes

+7 more reviewers

@RossBrunton RossBrunton RossBrunton left review comments

@frasercrmck frasercrmck frasercrmck left review comments

@kbenzie kbenzie kbenzie left review comments

@GeorgeWeb GeorgeWeb GeorgeWeb approved these changes

@aarongreig aarongreig aarongreig approved these changes

@PietroGhg PietroGhg PietroGhg approved these changes

@steffenlarsen steffenlarsen steffenlarsen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues opencl OpenCL adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@martygrant @kbenzie @RossBrunton @frasercrmck @GeorgeWeb @nrspruit @aarongreig @PietroGhg @steffenlarsen